You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Paragraph dialog: indents in points win over indents in characters - #6186
Paragraphs can carry an indent in characters (w:firstLineChars, w:leftChars, w:rightChars, w:hangingChars) beside the same indent in twips. When both are there, the characters win, in Word and in WordCraft's layout. Documents saved by WPS often have both, for example w:firstLineChars="400" w:firstLine="1417".
The Paragraph dialog's OK (para.set) set the indents in points and left the characters in place. Setting a first line of 2.5 cm on such a paragraph wrote w:firstLine="1417", but the paragraph kept its 4 characters, on the page and after a save. To the user, the dialog seemed to forget the value.
The other indent commands already handle this. para.indents (the ruler), Increase/Decrease Indent, Tab at the start of a paragraph, Backspace and the hanging indent shortcuts all call ParaProps::cancel_char_indents. Only para.set didn't.
The change
set_props now cancels the indents in characters for each indent it sets in points: left, right and first line, from props or from onPage. It skips one when props also gives that indent in characters. As in cancel_line_spacing, it changes only paragraphs where the resolved value (the paragraph's own or its style's) is non-zero. Paragraphs without indents in characters aren't touched, so nothing extra is written for them.
Testing
para_set_indents_in_points_win_over_indents_in_characters covers a paragraph with 4 characters beside 70.85 pt set to 2.5 cm, a paragraph without characters, a style's indentLeftChars overridden through onPage, and characters given along with the points. It fails with the fix taken out.
fmt, clippy for the engine, and the tests for engine, docx and layout pass locally on Windows, on top of main2922fe5.
I checked that typing a value in the dialog's By box and pressing OK or Enter at once commits it (a kittest case, not included), so the dialog side wasn't the cause.
Point-based paragraph indents now take precedence over existing character-based indents. Character indents explicitly set alongside point-based values are preserved.
Set Paragraph Formatting (the Paragraph dialog's OK) set the indents in
points but left a paragraph's w:firstLineChars (or its style's), which
win in Word and in the layout, so a first line set to 2.5 cm in a WPS
document stayed four characters. It now cancels them, as Word writes
w:firstLineChars=0, for the indents it sets in points and only where
the paragraph has indents in characters.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No actionable comments were generated in the recent review.
Walkthrough
When para.set applies point-based paragraph indents, it clears conflicting resolved character indents unless the matching character indent is also supplied. A test covers direct, style-based, and explicitly supplied character indents.
Changes: Paragraph indent precedence
Layer / File(s)
Summary
Apply and validate indent precedence crates/engine/src/cmd/para/format.rs, crates/engine/src/tests.rs
set_props clears nonzero resolved character indents when a point-based value is present and the matching character value is absent from props. The test checks direct and style-based values, plus a character value supplied alongside a point value.
The reviewed indent-precedence change has no identified issue requiring a fix before merge.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly summarizes the main change: point-based paragraph indents take precedence over character-based indents.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.)
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 2922fe5 and 0eb6f85.
Files selected for processing: 2
crates/engine/src/cmd/para/format.rs
crates/engine/src/tests.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Paragraphs can carry an indent in characters (
w:firstLineChars,w:leftChars,w:rightChars,w:hangingChars) beside the same indent in twips. When both are there, the characters win, in Word and in WordCraft's layout. Documents saved by WPS often have both, for examplew:firstLineChars="400" w:firstLine="1417".The Paragraph dialog's OK (
para.set) set the indents in points and left the characters in place. Setting a first line of 2.5 cm on such a paragraph wrotew:firstLine="1417", but the paragraph kept its 4 characters, on the page and after a save. To the user, the dialog seemed to forget the value.The other indent commands already handle this.
para.indents(the ruler), Increase/Decrease Indent, Tab at the start of a paragraph, Backspace and the hanging indent shortcuts all callParaProps::cancel_char_indents. Onlypara.setdidn't.The change
set_propsnow cancels the indents in characters for each indent it sets in points: left, right and first line, frompropsor fromonPage. It skips one whenpropsalso gives that indent in characters. As incancel_line_spacing, it changes only paragraphs where the resolved value (the paragraph's own or its style's) is non-zero. Paragraphs without indents in characters aren't touched, so nothing extra is written for them.Testing
para_set_indents_in_points_win_over_indents_in_characterscovers a paragraph with 4 characters beside 70.85 pt set to 2.5 cm, a paragraph without characters, a style'sindentLeftCharsoverridden throughonPage, and characters given along with the points. It fails with the fix taken out.fmt,clippyfor the engine, and the tests for engine, docx and layout pass locally on Windows, on top ofmain2922fe5.kittestcase, not included), so the dialog side wasn't the cause.Overlaps
styles.modify, for the Modify Style dialog. It doesn't touchpara.set, so the two don't overlap.set_props, to cancel spacing in lines, not indents. It's a different part of the function, though the two may need a small merge.🤖 Generated with Claude Code
Summary by CodeRabbit